Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8600629232
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4d18c41 to
7cbf210
Compare
jif-oai
left a comment
There was a problem hiding this comment.
Mostly testing so I pre-approve
|
|
||
| shell_command = shlex.join(args.command) | ||
| sandbox_command = shlex.join( | ||
| ["codex", "sandbox", "-P", profile_name, "--", "bash", "-lc", shell_command] |
There was a problem hiding this comment.
This isn’t trusted input here... The model can prefix the wrapper invocation with CODEX_PERMISSION_PROFILE=:danger-full-access. zsh-fork forwards that environment into the unsandboxed helper, and this code then selects the attacker-chosen remote profile
Can we inject the profile at the trusted escalation boundary, or keep this variable strictly informational and drop the enforcement example?
There was a problem hiding this comment.
Yes, everything you say is true: as noted in the PR body:
The new value is intentionally informational: child processes can overwrite environment variables, so consumers must decide whether their process-tree context is trusted before using it.
As for this:
Can we inject the profile at the trusted escalation boundary, or keep this variable strictly informational and drop the enforcement example?
Yes, I would like to introduce a more authoritative mechanism, but I think it's going to take some work to figure out how to do that. I suspect we will have to use OS-specific APIs to achieve this, but for now, this will at least give users something to play with.
There was a problem hiding this comment.
@jif-oai Though to your point, I updated the example to specify a ALLOWED_PROFILES just like it has an ALLOWED_HOSTS to illustrate that CODEX_PERMISSION_PROFILE should be checked rather than silently passed through.
|
|
||
| shell_command = shlex.join(args.command) | ||
| sandbox_command = shlex.join( | ||
| ["codex", "sandbox", "-P", profile_name, "--", "bash", "-lc", shell_command] |
There was a problem hiding this comment.
And doesn't the -P ignores the managed requirements?
There was a problem hiding this comment.
Great catch! Though we have a --include-managed-config option to exercise cloud requirements, so I'll update the PR body and code to include this.
3e056d3 to
cf7fb2d
Compare
tl;dr
Inject a
CODEX_PERMISSION_PROFILEenvironment variable with the name of the current permission profile when invoking a shell tool.Why
Shell tool owners may need to launch nested commands under the same named permission profile, including through
codex sandbox -P PROFILE --include-managed-config. Until now, child processes could observe sandbox and network metadata but could not identify the active named permission profile.The
--include-managed-configflag is essential when a helper reconstructs the sandbox from a profile name: it ensures the nested sandbox also loads managed enterprise requirements. Without it, using the inherited profile could unintentionally create a sandbox that does not enforce the organization's managed restrictions.The new environment value is intentionally informational and must not be treated as trusted input. Any process in the ancestry can overwrite an environment variable, so a consumer that passes this value to
codex sandbox -Pmust first validate it against the profiles that helper is authorized to use.Example Use Case
Suppose an organization provides a trusted
remote-bashwrapper that lets Codex run a command on an approved build host. The local shell command uses the named:workspacepermission profile:The command exposed to the model is a small zsh wrapper. It deliberately delegates with
exec, preserving the original arguments and process environment:The model invokes the public wrapper, not its Python implementation:
Only the inner implementation is authorized to escape the local sandbox:
With zsh-fork, execution begins with
remote-bashinside the:workspacesandbox. When the wrapper callsexec, the exact prefix rule matchesremote_bash.py, so that inner script is restarted unsandboxed. The escalated process inherits:Inheritance does not make the value trustworthy.
remote_bash.pyindependently allowlists both the remote host and the permission profile before using either value. In particular, a forged value such as:danger-full-accessis rejected before it can reachcodex sandbox -P:This builds each command layer as an argument vector and uses
shlex.join()at the boundary, rather than interpolating untrusted shell text. After validation and parsing, the nested command has this structure:A production implementation could execute that SSH command. The integration fixture prints it and parses the result back into arguments, verifying the complete flow:
This gives the trusted helper access to resources outside the local sandbox—such as SSH credentials—while ensuring that it can select only an explicitly authorized profile and that work on the remote host remains subject to the organization's managed requirements.
What changed
CODEX_PERMISSION_PROFILEafter shell environment policy evaluation so the active profile wins over inherited or configured stale values.shell_commandand unifiedexec_command, including local, zsh-fork, and remote exec-server paths.Testing
require_escalatedscript outside the:workspacesandbox while preservingCODEX_PERMISSION_PROFILE=:workspace.remote_bash.pyruns unsandboxed, and its printed SSH command reconstructs the inherited:workspacesandbox with--include-managed-configwhile preserving every argument after--.CODEX_PERMISSION_PROFILEas untrusted and validates it againstALLOWED_PROFILESbefore constructing the nested command.--include-managed-configso nested use of the inherited profile cannot bypass managed enterprise requirements.shell_commandreceives the selected active profile.printenv CODEX_PERMISSION_PROFILE.